Repository navigation
fix(roles): ignore server-generated ids when comparing roles - #1674
RutgerLubbers wants to merge 2 commits into
Conversation
Role change-detection compared the server-assigned `id` and `containerId`, so a realm exported from another Keycloak instance made every role look changed and triggered pointless updates. Since Keycloak 26.6.4 those updates fail with HTTP 403 on the `admin`, `create-realm` and `realm-management` roles, aborting the import. Both identifiers are now excluded from the patch and the equality check for realm and client roles, matching what ClientImportService already does. The existing role's identifiers survive because the patch is built on a clone of it.
|
| RoleRepresentation patchedRole = CloneUtil.patch(existingRole, roleToImport, propertiesToIgnoreOnUpdate); | ||
| if (roleToImport.getAttributes() != null) { |
There was a problem hiding this comment.
I disagree that this change is the cause of the underlying analysis.
Further the method is only called if existingRole != null and CloneUtil.patch only returns null if origin (or existingRole in this case) is null.
| RoleRepresentation patchedRole = CloneUtil.patch(existingRole, roleToImport, propertiesToIgnoreOnUpdate); | ||
| String roleName = existingRole.getName(); |
Motouom
left a comment
Motouom
left a comment
There was a problem hiding this comment.
Hello @RutgerLubbers, really sorry for the late reply.
Thanks for the implementation. I reviewed the role comparison change and tested it locally.
The code-level behavior looks correct: server-generated id and containerId are ignored for comparisons, genuine role content changes are still applied, and the target instance IDs are preserved. The focused tests and full ./mvnw clean install passed, including Checkstyle, PMD, SpotBugs, and license checks. I also verified the JSON fixtures through the compiled kccli JAR against Docker Keycloak and confirmed the realm/client roles in the Admin Console.
Before this can be approved and merged, please:
- Resolve the current merge conflicts with
adorsys/main. - Re-run the complete CI pipeline on the updated commit.
- Confirm that the current Sonar/quality gate is green; the PR still has a historical Sonar failure recorded.
I did not find an additional functional code defect, but the conflicts and quality-gate status still block approval.
|
|
@Motouom Is this good to go like this? |
Role change-detection compared the server-assigned
idandcontainerId, so a realm exported from another Keycloak instance made every role look changed and triggered pointless updates. Since Keycloak 26.6.4 those updates fail with HTTP 403 on theadmin,create-realmandrealm-managementroles, aborting the import.Both identifiers are now excluded from the patch and the equality check for realm and client roles, matching what ClientImportService already does. The existing role's identifiers survive because the patch is built on a clone of it.
Fixes #1672